Add Flutter lint #253

jasikpark · 2025-02-13T17:28:23Z

Add https://pub.dev/packages/flutter_lints and fix the complaining lints

See https://dart.dev/tools/analysis#lints for more

This surfaced two package imports that weren't tracked by pubspec.yaml: path was imported w/o being registered as a dep, and flutter_web_plugins, an internal tool of Flutter, was being imported, and needed a manually added dependency.

IanVS · 2025-02-27T16:29:46Z

lib/main.dart

 import 'package:mobile_nebula/services/theme.dart';
 import 'package:mobile_nebula/services/utils.dart';
 import 'package:sentry_flutter/sentry_flutter.dart';

 Future<void> main() async {
-  usePathUrlStrategy();


According to git blame (https://github.com/DefinedNet/mobile_nebula/blame/bcfcadec8ef46f2ed628ab9334088e1f142f22a6/lib/main.dart#L18), this was added as part of the work to support DN enrollment. We need to be able to process a url deep-link when enrolling, so I do not think we can remove this package or this line.

BTW I think this is the only real blocker here. (And maybe the concatenation one, depending on whether that format actually works or not in dart.)

iOS at least works with this removed, the correct page is opened.

I believe https://github.com/stryder-dev/flutter_platform_widgets/wiki/Routing may handle this instead, not requiring importing an un-exported function.

mobile_nebula/lib/main.dart

Lines 92 to 126 in bcfcade

(context) => PlatformApp(

debugShowCheckedModeBanner: false,

localizationsDelegates: <LocalizationsDelegate<dynamic>>[

DefaultMaterialLocalizations.delegate,

DefaultWidgetsLocalizations.delegate,

DefaultCupertinoLocalizations.delegate,

],

title: 'Nebula',

material: (_, __) {

return new MaterialAppData(

themeMode: brightness == Brightness.light ? ThemeMode.light : ThemeMode.dark,

theme: brightness == Brightness.light ? theme.light() : theme.dark(),

);

},

cupertino: (_, __) => CupertinoAppData(theme: CupertinoThemeData(brightness: brightness)),

onGenerateRoute: (settings) {

if (settings.name == '/') {

return platformPageRoute(context: context, builder: (context) => MainScreen(this.dnEnrolled));

}

final uri = Uri.parse(settings.name!);

if (uri.path == EnrollmentScreen.routeName) {

// TODO: maybe implement this as a dialog instead of a page, you can stack multiple enrollment screens which is annoying in dev

return platformPageRoute(

context: context,

builder:

(context) =>

EnrollmentScreen(code: EnrollmentScreen.parseCode(settings.name!), stream: this.dnEnrolled),

);

}

return null;

},

),

);

What do you mean by "importing an un-exported function"? What is being done here is what the docs show: https://docs.flutter.dev/ui/navigation/url-strategies#configuring-the-url-strategy

trying to get my android emulator to work for testing...

ah, appreciate that the deeplink validator works...

What do you mean by "importing an un-exported function"? What is being done here is what the docs show: docs.flutter.dev/ui/navigation/url-strategies#configuring-the-url-strategy

We needed to add the package as a direct dependency, rather than indirectly importing it as part of the flutter sdk.

Added that dep to pubspec.yaml and everything's all good now.

IanVS · 2025-02-27T16:47:48Z

lib/services/theme.dart

@@ -11,51 +11,51 @@ class MaterialTheme {
  static ColorScheme lightScheme() {
    return const ColorScheme(
      brightness: Brightness.light,
-      primary: Color(4284700303),


I wonder if we should ignore this file, since it was auto-generated and it will be easier to compare changes in the future if we don't muck around with it too much.

IanVS · 2025-02-27T19:18:46Z

lib/services/utils.dart

@@ -41,10 +41,10 @@ class Utils {

  static String itemCountFormat(int items, {singleSuffix = "item", multiSuffix = "items"}) {
    if (items == 1) {
-      return items.toString() + " " + singleSuffix;
+      return "$items " + singleSuffix;


This is still using concatenation, and does it need to be ${items}? (same below)

Yep, this syntax works ${...} syntax is only needed for complex expressions, so ${items+1} v.s.$items.

https://learnxbyexample.com/dart/text-templates/

Okay, but, shouldn't this still be `${items} ${singleSuffix}`?

yes, whoops forgot to actually augment the autofix.

❯ jj absorb Absorbed changes into these revisions: povmpvpx 93f7cc15 `dart fix --apply --code=prefer_interpolation_to_compose_strings` Rebased 10 descendant commits. Working copy now at: wusutqos b0d3d23d (no description set) Parent commit : pxwtotsz 05879848 flutter-lint* | `dart format lib/` Remaining changes: M lib/services/utils.dart

Love that I can use jj absorb to automatically include the fix in the associated commit it should have been in. Works 70% of the time 100% of the time!

IanVS · 2025-02-27T19:23:50Z

lib/services/settings.dart

@@ -11,7 +11,7 @@ bool DEFAULT_TRACK_ERRORS = true;
 class Settings {
  final _storage = Storage();
  final StreamController _change = StreamController.broadcast();
-  var _settings = Map<String, dynamic>();
+  var _settings = <String, dynamic>{};


Not a huge fan of this rule, seems stylistic and more obtuse, but if that's the recommended way, I guess we can roll with it. Just seems like there's no good reason not to be explicit. /shrug

I'll leave it enabled. not a fan, but i like keeping the config to a min over customizing.

https://docs.flutter.dev/ui/navigation/url-strategies#configuring-the-url-strategy

jasikpark force-pushed the flutter-lint branch 3 times, most recently from 408c287 to 0f457d3 Compare February 13, 2025 20:20

jasikpark force-pushed the flutter-fmt branch from 852984c to 0cba11a Compare February 13, 2025 20:20

jasikpark force-pushed the flutter-lint branch 2 times, most recently from e32b93e to 317c607 Compare February 13, 2025 20:54

jasikpark force-pushed the flutter-fmt branch from 0cba11a to d317e21 Compare February 13, 2025 20:54

Base automatically changed from flutter-fmt to main February 13, 2025 21:37

jasikpark force-pushed the flutter-lint branch from 317c607 to a2f61a8 Compare February 13, 2025 21:40

jasikpark marked this pull request as ready for review February 13, 2025 21:40

jasikpark requested review from IanVS and johnmaguire February 13, 2025 21:41

jasikpark added 5 commits February 26, 2025 12:30

Enable flutter_lints linting

95f217e

Fix unmarked deps, we aren't on web so we don't need a URL strategy

9030664

dart fix --apply --code=use_super_parameters

7b0441a

dart fix --apply --code=use_key_in_widget_constructors

ca4f55c

dart fix --apply --code=use_function_type_syntax_for_parameters

c88014c

jasikpark force-pushed the flutter-lint branch from a2f61a8 to 8a3f628 Compare February 26, 2025 18:30

IanVS reviewed Feb 27, 2025

View reviewed changes

jasikpark added 8 commits March 3, 2025 18:07

Ignore code-generated lib/services/theme.dart file

78157a8

dart fix --apply --code=unnecessary_this

65d1ecd

dart fix --apply --code=unnecessary_null_in_if_null_operators

e6e7775

dart fix --apply --code=unnecessary_new

73f1191

dart fix --apply --code=sort_child_properties_last

38c9cfc

dart fix --apply --code=sized_box_for_whitespace

c01c0ed

dart fix --apply --code=prefer_typing_uninitialized_variables

a3387ea

dart fix --apply --code=prefer_is_empty

9b4c1be

jasikpark force-pushed the flutter-lint branch from 8a3f628 to c8dcc70 Compare March 4, 2025 00:29

jasikpark requested a review from IanVS March 4, 2025 00:29

dart fix --apply --code=prefer_interpolation_to_compose_strings

f58b374

jasikpark added 9 commits March 4, 2025 10:09

dart fix --apply --code=prefer_final_fields

cbcf435

dart fix --apply --code=prefer_const_constructors_in_immutables

99e3b4a

dart fix --apply --code=prefer_collection_literals

67e8896

dart fix --apply --code=no_leading_underscores_for_local_identifiers

45f0b38

dart fix --apply --code=curly_braces_in_flow_control_structures

581c2f3

dart fix --apply --code=avoid_function_literals_in_foreach_calls

89974a5

dart fix --apply --code=annotate_overrides

e6c1f19

Add CI for dart linting

1b13646

dart format lib/

2d0ba84

jasikpark force-pushed the flutter-lint branch from c8dcc70 to 2d0ba84 Compare March 4, 2025 16:10

Re-enable the usePathUrlStrategy call, with proper deps

e6f0929

https://docs.flutter.dev/ui/navigation/url-strategies#configuring-the-url-strategy

IanVS approved these changes Mar 4, 2025

View reviewed changes

jasikpark merged commit 2b844d2 into main Mar 4, 2025
5 checks passed

jasikpark deleted the flutter-lint branch March 4, 2025 17:29

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Flutter lint #253

Add Flutter lint #253

jasikpark commented Feb 13, 2025 •

edited

Loading

IanVS Feb 27, 2025

IanVS Feb 28, 2025 •

edited

Loading

jasikpark Mar 4, 2025

jasikpark Mar 4, 2025

IanVS Mar 4, 2025

jasikpark Mar 4, 2025

jasikpark Mar 4, 2025

jasikpark Mar 4, 2025

IanVS Feb 27, 2025

jasikpark Mar 4, 2025

IanVS Feb 27, 2025

jasikpark Mar 4, 2025

IanVS Mar 4, 2025 •

edited

Loading

jasikpark Mar 4, 2025

jasikpark Mar 4, 2025

IanVS Feb 27, 2025

jasikpark Mar 4, 2025

	(context) => PlatformApp(
	debugShowCheckedModeBanner: false,
	localizationsDelegates: <LocalizationsDelegate<dynamic>>[
	DefaultMaterialLocalizations.delegate,
	DefaultWidgetsLocalizations.delegate,
	DefaultCupertinoLocalizations.delegate,
	],
	title: 'Nebula',
	material: (_, __) {
	return new MaterialAppData(
	themeMode: brightness == Brightness.light ? ThemeMode.light : ThemeMode.dark,
	theme: brightness == Brightness.light ? theme.light() : theme.dark(),
	);
	},
	cupertino: (_, __) => CupertinoAppData(theme: CupertinoThemeData(brightness: brightness)),
	onGenerateRoute: (settings) {
	if (settings.name == '/') {
	return platformPageRoute(context: context, builder: (context) => MainScreen(this.dnEnrolled));
	}

	final uri = Uri.parse(settings.name!);
	if (uri.path == EnrollmentScreen.routeName) {
	// TODO: maybe implement this as a dialog instead of a page, you can stack multiple enrollment screens which is annoying in dev
	return platformPageRoute(
	context: context,
	builder:
	(context) =>
	EnrollmentScreen(code: EnrollmentScreen.parseCode(settings.name!), stream: this.dnEnrolled),
	);
	}

	return null;
	},
	),
	);

Add Flutter lint #253

Add Flutter lint #253

Conversation

jasikpark commented Feb 13, 2025 • edited Loading

Choose a reason for hiding this comment

IanVS Feb 28, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

IanVS Mar 4, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jasikpark commented Feb 13, 2025 •

edited

Loading

IanVS Feb 28, 2025 •

edited

Loading

IanVS Mar 4, 2025 •

edited

Loading